This is all beside the point, but I was looking into abstracting the stylebook component further so it can be dropped into the edit and view contexts without knowing too much about its own props.

https://github.com/WordPress/gutenberg/pull/67660/files#diff-ecd3155ef49b2da19da151cb9ac6813db5a887e4e7bd4ab1ca44b41ddba97b8b

I mean, this works too 😄

I'm wondering if long term we'll still need the style book in the edit context at all.

Reading through #66719, it seems the main reason global styles should stay in that right hand side sidebar in the editor is so folks can tweak styles while building their templates. While revisions should probably be accessible from wherever global styles can be edited, the style book is a more high-level tool so I'd question the usefulness of keeping it in that right hand sidebar.

I'm wondering if long term we'll still need the style book in the edit context at all.

👍🏻 I'd be for this if only to simplify the all the edit canvas view juggling etc we do.

Do we need a second stylebook route?

What I'm wondering is if /styles could work with sub-routes to make it more flexible later if further subroutes are added (I'm thinking revisions).

The route would end up looking like:

?p=/styles/style-book

	areas: {
		content( { query, params } ) {
			console.log( { query, params } );
			// params.subRoute, e.g., `style-book`
			// query.section, e.g., `/typography`
                        // Now handle the logic in the component as per usual
			return (
				<GlobalStylesUIWrapper
					subRoute={ params.subRoute }
					section={ query.section }
				/>
			);
		},

Revisions is also meant to work with the style book though? How would that look if they're both sub-routes of styles? Could revisions instead be a sub-route of either styles or styles-stylebook?

I don't feel strongly about either option. Mostly at this point I'm thinking about the further splitting up of stylebook into a few sub-routes equivalent to the tabs in the older version. But that should still work if stylebook is itself a sub-route so 🤷

I think we need to think about this more holistically. The way I think about routes is that I try to map these to navigation menus:

• The "root" sidebar of the site editor should represent the top level routes /some-top-level-section
• If there's a sub-menu within one of these section, I should create a sub route /top-level/second-level
• If there's multiple "modifiers" that are not necessarily a navigation menu (like switching the "mode" or "layout" or things like that) and we want this state to be url accessible, I should be using "query") /top-level/second-level?modifier=something

Based on this, you should be able to make the right call. I'm not wrong, today the "style book" is part of a dropdown where we switch what to "visualize" when editing styles. So it seems close to me to a "modifier" than a "submenu", so I'd probably use query: /styles?preview=style-book or something.

Ok trying to think this through.

What we have right now is a top level route at the root of the site editor: /site-editor.php and then we're using queries for everything else, e.g. Pages: site-editor.php?p=%2Fpage, Styles: site-editor.php?p=%2Fstyles.

We kind of have submenus already in that within Pages we have Published, Drafts etc. I guess within Styles we could consider the main global styles sections: Colors, Typography etc. to also be submenus. But we're not using /top-level/second-level URL structure for those, we're just using query parameters.

Do we plan to change the overall URL structure for these pages anytime soon? Otherwise, keeping to the logic of the current structure, both submenus and modifiers should be expressed as query parameters.

Does that make sense, or am I missing anything here? I'm not really invested in any solution, just trying to understand what will work best overall!

What we have right now is a top level route at the root of the site editor: /site-editor.php and then we're using queries for everything else, e.g. Pages: site-editor.php?p=%2Fpage, Styles: site-editor.php?p=%2Fstyles.

I wouldn't consider the site-editor.php as top level route, It's just a temporary technical limitation while we iterate on admin redesign. In fact we already explored pretty permalinks in the admin and we know it's possible.

Instead consider p as the "path" and the rest as query. This is exactly how the router works btw.

The way I think about routes is that I try to map these to navigation menu
Instead consider p as the "path" and the rest as query. This is exactly how the router works btw.

Thanks for sharing that. Make sense to me.

So it seems close to me to a "modifier" than a "submenu", so I'd probably use query: /styles?preview=style-book or something.

➕

Ok I've updated to use a query instead.

I think these replacements aren't quite right, as they don't take into account any subroutes or params of /styles, e.g., /styles?section=%2Fvariations

To test, navigation to styles > browse styles, then activate the style book. Try to toggle it off or navigation back to the main menu.

Would adding and removing the preview query arg be enough?

				const updatedPath = ! isStyleBookOpened
					? stylebookPath
					: removeQueryArgs( path, 'preview' );

Oooh maybe, let me try!

Yes it worked 😄 thanks!

Something about this hook placement triggers the following warning:

Cannot update a component (`BlockPreview`) while rendering a different component (`StyleBookPreview`). To locate the bad setState() call inside `StyleBookPreview`

For example, when navigating to an individual block's global style controls, and activating the style book.

It means we might have to wrap the dispatch in a useEffect so the settings are updated after render:

	useEffect( () => {
		dispatch( blockEditorStore ).updateSettings( siteEditorSettings );
	}, [ siteEditorSettings ] );

But I'm unsure of the side-effects - will it preserve the original intention to get the updated colors?

Hmm that was merged in the classic stylebook PR though I'm not seeing the same error on trunk. I'll investigate.

Ok so the error is caused by this PR rendering the StyleBookPreview component simultaneously with the global styles sidebar, and it seems to be complaining about the fact that we're updating the block editor settings inside StyleBookPreview while at the same time using those settings to render the BlockPreview inside the global styles sidebar. That's why the error only occurs when a specific block is selected in global styles and we click over to the stylebook view.

Wrapping the settings update in a useEffect does seem to fix the issue, so I'll add that in for now.

What I don't understand though is why we need to do this settings update in here at all. With the stylebook that renders inside the editor, the block editor settings are already up to date, but I can't work out whereabouts they're being updated.

Another thing to consider is: should we be depending on the block editor settings here, or the site editor ones? Perhaps instead of using useMultipleOriginColorsAndGradients we should have an equivalent function that grabs the color palettes from the site editor settings?

What I don't understand though is why we need to do this settings update in here at all.

I'm not sure either 🤔 I assumed it's because useMultiOriginPalettes needed the updated values here, but I commented the update line out and can't discern any difference when I update the palette in global styles and apply it to a block (the style book updates still).

Wasn't the update added as part of the class style book? #66851

I am missing something, that much is clear.

Another thing to consider is: should we be depending on the block editor settings here, or the site editor ones?

I'd say site settings is right, because the site editor is initialized (and therefore the site editor store) with preloaded theme styles settings.

Perhaps instead of using useMultipleOriginColorsAndGradients we should have an equivalent function that grabs the color palettes from the site editor settings?

Conceptually, that sounds legit to me, but I haven't gone deeper than inspecting useMultipleOriginColorsAndGradients and useSettings . The context is always going to be global styles/theme json, right?

The context is always going to be global styles/theme json, right?

It goes a bit beyond theme.json because for classic themes it also picks up anything defined in the editor-color-palette and editor-gradient-presets theme supports. But that's when those palettes are in the block editor settings already.

I think it might be that the block editor settings only get updated when an actual block editor renders. In which case, if we're in the site preview, it makes sense that block editor settings aren't up to date, but site editor settings (which are essentially the same shape) already are.